Increase maxPerformDataSize for maxObservation, maxOutcome size #292
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
infiloop2
commented
Nov 27, 2023
| // which is guaranteed onchain | ||
| MaxOutcomeLength = 2_000_000 | ||
| MaxOutcomeLength = 2_500_000 |
There was a problem hiding this comment.
this is a smaller change than 2x because
- there was existing margin
- there is a component of proposals in outcome that didn't scale according to performData
infiloop2
changed the title
amirylm
approved these changes
Nov 27, 2023
infiloop2
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
maxPerformDataSize was increased to 10000 from 5000, it breaks current maxObservation and maxOutcome sizes
For maxObservationSize, I had to reduce the number of allowed upkeeps instead of increasing observation size as libocr puts a limit of 1Mb. This should be ok when new nodes will send less number of upkeeps that old nodes should be able to parse. however on new nodes we do validations on number of performables from old nodes' observations, which will break if it goes above 50. Our current plan is that we don't expect it will be hit in prod during upgrade.
For maxOutcomeSize, i could increase it to fit the given upkeeps. This should also be ok as it increases max outcome size, old nodes outcome would still be valid. If new nodes outcome size increases 2Mb, then old nodes won't be able to handle it but that was broken anyway
A more robust solution is to calculate this limit dynamically but that is considerably more involved. created auto-7597 for it
Test Plan: Will rely on existing CI tests and node upgrade tests, with known expectation that upgrade will fail if there are more than 50 upkeeps eligible per second during upgrade